Skip to content

Fix: Honor MSA tenant GUID when specified via WithTenantId at request level#5958

Merged
Robbie-Microsoft merged 10 commits into
mainfrom
rginsburg/withTenantId_bugfix
May 13, 2026
Merged

Fix: Honor MSA tenant GUID when specified via WithTenantId at request level#5958
Robbie-Microsoft merged 10 commits into
mainfrom
rginsburg/withTenantId_bugfix

Conversation

@Robbie-Microsoft

@Robbie-Microsoft Robbie-Microsoft commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #5951

When a ConfidentialClientApplication is configured with a specific tenant at the app level, calling .WithTenantId("9188040d-6c67-4c5b-b112-36a304b66dad") (the MSA/consumer tenant GUID) or .WithTenantId("consumers") at request level was silently ignored — the app-level authority was used instead.

Root Cause

IsCommonOrganizationsOrConsumersTenant() incorrectly treated "consumers" (and the MSA tenant GUID it resolves to) as tenantless aliases on par with "common" / "organizations". This caused request-level tenant overrides using either form to be discarded.

The correct distinction is:

  • "common" / "organizations" → genuinely tenantless; request-level override should be ignored
  • "consumers" / "9188040d-6c67-4c5b-b112-36a304b66dad" → real tenant identifiers; should be honored at request level

Changes

AadAuthority.cs

  • Deleted IsCommonOrganizationsOrConsumersTenant() (both instance and static overloads) — the misleading helper that conflated tenantless aliases with real tenant IDs
  • Removed "consumers" from s_tenantlessTenantNames — only "common" and "organizations" are truly tenantless
  • Fixed GetTenantedAuthority() to use IsCommonOrOrganizationsTenant() so the "consumers" alias and MSA GUID are preserved rather than substituted

AuthorityInfo.cs

Fixed CreateAuthorityForRequestAsync to use IsCommonOrOrganizationsTenant() — request-level "consumers" and MSA GUID are now both honored as real tenants. Updated XML doc comment.

MtlsPopParametersInitializer.cs

Fixed the mTLS PoP validation that rejects non-tenanted authorities: replaced IsCommonOrganizationsOrConsumersTenant with IsCommonOrOrganizationsTenant so apps using the MSA GUID as their authority are no longer incorrectly rejected.

TokenCache.ITokenCacheInternal.cs

Fixed OBO cache filtering: replaced IsCommonOrganizationsOrConsumersTenant with IsCommonOrOrganizationsTenant so the MSA GUID is treated as a real tenanted authority and tenant-based cache filtering is applied correctly.

CiamAuthority.cs / DstsAuthority.cs

Replaced IsCommonOrganizationsOrConsumersTenant with IsCommonOrOrganizationsTenant in GetTenantedAuthority overrides.

Tests

AuthorityTests.csWithTenantId_ConsumerGuid_IsHonoredAtRequestLevel

# Config authority Request authority Expected Purpose
1 specific tenant MSA GUID MSA GUID wins Bug regression
2 common MSA GUID MSA GUID wins Common → GUID honored
3 specific tenant "consumers" string consumers wins Full relaxation
4 common "consumers" string consumers wins Full relaxation
5 MSA GUID (app level) none MSA GUID used No regression

AadAuthorityTests.cs

Removed IsCommonOrganizationsOrConsumersTenant_MsaGuid_ReturnsFalse — the helper no longer exists.

… level

When a ConfidentialClientApplication is configured with a specific tenant
at the app level, calling .WithTenantId('9188040d-...') (the MSA/consumer
tenant GUID) at the request level was silently ignored.

Root cause: IsCommonOrganizationsOrConsumersTenant() equates both the
'consumers' string alias and the MSA GUID as tenantless, causing the
request-level override to be discarded.

Fix: Add IsConsumersGuid() helper to AadAuthority that returns true only
for the MSA GUID (not the 'consumers' alias). Use it in the request
authority resolution condition in CreateAuthorityForRequestAsync so the
GUID is honored as a real tenant ID at request level.

The 'consumers' string alias continues to be ignored at request level
per the existing known limitation (#2929).

Fixes #5951

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Robbie-Microsoft Robbie-Microsoft requested a review from a team as a code owner April 28, 2026 17:45
Copilot AI review requested due to automatic review settings April 28, 2026 17:45
…iltering

GetTenantedAuthority (AadAuthority): the MSA tenant GUID is a real tenant ID
and should not be replaced by a caller-supplied tenant when forceSpecifiedTenant=false.
Previously IsCommonOrganizationsOrConsumersTenant() returned true for the GUID,
causing it to be replaced like 'common' or 'consumers'. Added && !IsConsumersGuid()
guard to preserve the GUID unless replacement is explicitly forced.

FilterTokensByHomeAccountTenantOrAssertion (OBO cache): the same root cause meant
OBO cache lookups with the MSA GUID as authority set filterByTenantId=false, logging
a spurious 'Please use tenanted authority' warning and skipping tenant filtering.
Fixed by treating the MSA GUID as a tenanted authority in the filter expression.

Test: GetTenantedAuthority_MsaGuid_IsNotReplaced in AadAuthorityTests covers both
forceSpecifiedTenant=false (preserved) and =true (replaced) cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs Outdated
Comment thread tests/Microsoft.Identity.Test.Unit/PublicApiTests/MtlsPopTests.cs Outdated
Robbie-Microsoft and others added 2 commits May 1, 2026 13:26
The MSA tenant GUID (9188040d-6c67-4c5b-b112-36a304b66dad) is a real,
concrete tenant ID — not a tenantless alias like 'consumers', 'common',
or 'organizations'. The previous fix added IsConsumersGuid() guards in
four places; this commit fixes the predicate itself instead.

Changes:
- AadAuthority.IsCommonOrganizationsOrConsumersTenant: replace
  IsConsumers(tenantId) with a plain 'consumers' string equality check,
  so only the alias string is tenantless — not the GUID
- Remove IsConsumersGuid() instance and static helpers (no longer needed)
- Remove the four IsConsumersGuid() guards that compensated for the
  incorrect predicate behavior
- Add predicate unit test asserting MSA GUID returns false
- Add Bearer token mocked HTTP test (issue #5951) asserting the token
  request targets the MSA tenant endpoint, not 'common'

Fixes #5951

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread tests/Microsoft.Identity.Test.Unit/ApiConfigTests/AuthorityTests.cs Outdated
neha-bhargava

This comment was marked as resolved.

Robbie-Microsoft and others added 2 commits May 7, 2026 12:58
- ConfidentialClientApplicationTests: change ClientCreds_WithTenantId_MsaGuid test
  to configure app with MSA GUID authority and override with a different tenant at
  request level, validating that the override fires (not the app-level MSA GUID)
- AuthorityTests: refactor WithTenantId_ConsumerGuid_IsHonoredAtRequestLevel from
  inline cases to [DataRow] with descriptive DisplayNames for easier reading in
  the test runner

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@neha-bhargava neha-bhargava left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks

bgavrilMS

This comment was marked as resolved.

Comment thread src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs Outdated
Robbie-Microsoft and others added 2 commits May 11, 2026 13:57
…equest-level tenant relaxation

- Delete the misleading IsCommonOrganizationsOrConsumersTenant helper (both instance
  and static overloads) — it conflated truly tenantless aliases (common/organizations)
  with real tenant identifiers (consumers/MSA GUID).
- Remove 'consumers' from s_tenantlessTenantNames to match the semantic model:
  only 'common' and 'organizations' are tenantless.
- Replace all 6 call sites with IsCommonOrOrganizationsTenant:
  AadAuthority.GetTenantedAuthority, AuthorityInfo.CreateAuthorityForRequestAsync,
  CiamAuthority.GetTenantedAuthority, DstsAuthority.GetTenantedAuthority,
  MtlsPopParametersInitializer, TokenCache.FilterTokensByHomeAccountTenantOrAssertion.
- Full relaxation: any request-level tenant that is not 'common' or 'organizations'
  (including 'consumers' string and the MSA GUID) is now honored at request level.
- Update AuthorityTests DataRows: consumers at request level now wins instead of
  being ignored (AppSpecificTenant_RequestConsumersAlias_ConsumersWins,
  AppCommon_RequestConsumersAlias_ConsumersWins).
- Delete IsCommonOrganizationsOrConsumersTenant_MsaGuid_ReturnsFalse test (method
  no longer exists).
- Delete MtlsPop_WithMsaTenantGuidAuthority_DoesNotThrowMissingTenantedAuthority
  test per Bogdan's review — covered by the bearer token regression test.

Fixes #5951

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetAuthorizationRequestUrl_WithConsumerInCreate_ReturnsConsumers was
asserting StartsWith(Constants.Common) but the test name says
'ReturnsConsumers'. The PR fix correctly routes consumers authority,
so update the assertion to match the intended behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs
Comment thread src/client/Microsoft.Identity.Client/Instance/CiamAuthority.cs
@bgavrilMS

Copy link
Copy Markdown
Member

LGTM

Robbie-Microsoft added a commit that referenced this pull request May 12, 2026
Remove 'consumers' from the list of tenants that should only be set at
app level. Since PR #5958, 'consumers' and the MSA GUID are real tenant
identifiers honored at request level; only 'common' and 'organizations'
are tenantless aliases that are ignored at request level.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Robbie-Microsoft Robbie-Microsoft merged commit 8a61acf into main May 13, 2026
15 checks passed
@Robbie-Microsoft Robbie-Microsoft deleted the rginsburg/withTenantId_bugfix branch May 13, 2026 17:34
Robbie-Microsoft added a commit that referenced this pull request May 28, 2026
… follow-up)

Follow-up to #5958 addressing reviewer feedback (#5958 (comment)):

- CiamAuthority: remove dead/unreachable code paths in GetTenantedAuthority.
- DstsAuthority: keep the silent-flow rewrite path that Authority.CreateAuthorityWithTenant
  relies on (tenantless dstsv2/common / dstsv2/organizations configured authorities
  must be rewritten to the tenanted form even when forceSpecifiedTenant is false), and
  document the contract in a comment. Adds two regression tests covering the
  non-forced rewrite for both `common` and `organizations`.
- AuthorityInfo: tighten an XML doc comment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] WithTenantId request-level override is ignored for consumer tenant

5 participants